Skip to content

fix(database): drop the join-without-index rules - #1255

Merged
kesselb merged 1 commit into
nextcloud:masterfrom
littleopa:fix/1245-select-full-join-notice
Oct 31, 2026
Merged

fix(database): drop the join-without-index rules#1255
kesselb merged 1 commit into
nextcloud:masterfrom
littleopa:fix/1245-select-full-join-notice

Conversation

@littleopa

@littleopa littleopa commented Oct 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

Select_full_join and Join_without_index_rate can't be acted on by an admin, so this removes both rules (as suggested in review):

  • The queries behind the counters aren't logged by default, so there is no way to tell which query did the join.
  • Most of them come from Nextcloud itself: the core setup checks for missing columns, indices, primary keys and bigint conversions introspect the schema through information_schema, whose tables have no indexes. Every visit of Administration → Overview adds 12 to Select_full_join (see the analysis in Database checks: Select_full_join warning is caused by the setup checks' own schema introspection #1245), so on a small instance just looking at the page makes the check fail.
  • Even if the query were known, adding an index by hand would make the schema check report it as unexpected.

Changes:

  • remove both rules from RuleSet\Mysql
  • remove them from Advisor::COUNTER_SENSITIVE_RULE_IDS
  • remove AdvisorTest::testJoinWithoutIndexRateIsANotice (and the now unused Rule import)

Seen on Nextcloud 35.0.1, serverinfo 7.0.0, MariaDB 10.11 (Debian 12), all occ db:add-missing-* checks clean.

Checklist

  • Code is properly formatted
  • Sign-off message is added to all commits
  • Tests are adjusted (the removed rule's test is dropped; no new behaviour to test)
  • Screenshots before/after for front-end changes (not applicable)
  • Documentation has been updated or is not required

AI (if applicable)

  • The content of this PR was partly or fully generated using AI (Claude Code, Assisted-by trailer on the commit)

🤖 Generated with Claude Code

https://claude.ai/code/session_019TNUHUT6jaQ3BHvjvdf4Ny

@kesselb

kesselb commented Oct 30, 2026

Copy link
Copy Markdown
Collaborator

Thanks for your patch 👍

Is my understanding correct that the warning/notice isn't useful because the admin cannot act on it? The query that joins without an index isn't logged by default, so the admin can't tell which one it is. And even if they found it, adding the index manually would make our schema checker report it as an unexpected index.

If that's the case, I would suggest updating this PR to remove both Select_full_join and Join_without_index_rate.

@littleopa
littleopa force-pushed the fix/1245-select-full-join-notice branch from d272da8 to 61ba92b Compare October 30, 2026 18:46
@littleopa @claude
fix(database): drop the join-without-index rules
53374c2 
Select_full_join and Join_without_index_rate cannot be acted on by an
admin:

- The queries behind the counters are not logged by default, so there is
  no way to tell which query did the join.
- Most of them are Nextcloud's own: the core setup checks introspect the
  schema through information_schema, whose tables have no indexes, so
  every visit of Administration -> Overview adds to Select_full_join.
- Even if the query were known, adding an index by hand would make the
  schema check report it as unexpected.

Remove both rules, as suggested in review.

Fixes nextcloud#1245

Assisted-by: ClaudeCode:claude-opus-5-5
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019TNUHUT6jaQ3BHvjvdf4Ny
Signed-off-by: Phillipxh <150742483+Phillipxh@users.noreply.github.com>
@littleopa
littleopa force-pushed the fix/1245-select-full-join-notice branch from 61ba92b to 53374c2 Compare October 30, 2026 21:36
@littleopa littleopa changed the title fix(database): downgrade the joins-without-index rule to a notice fix(database): drop the join-without-index rules Oct 30, 2026
@littleopa

littleopa commented Oct 30, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review, @kesselb! Yes, that's exactly it: the queries aren't logged by default, most of the full joins come from the core setup checks' own information_schema introspection, and a manually added index would show up as unexpected in the schema check.

I've updated the PR to remove both Select_full_join and Join_without_index_rate (rules, the COUNTER_SENSITIVE_RULE_IDS entries and the notice test), squashed into a single commit.

@kesselb

kesselb commented Oct 31, 2026

Copy link
Copy Markdown
Collaborator

/backport to stable35 please

@kesselb
kesselb merged commit 418bafb into nextcloud:master Oct 31, 2026
40 of 41 checks passed
@backportbot backportbot Bot mentioned this pull request Oct 31, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kesselb kesselb kesselb approved these changes
@benjaminfrueh benjaminfrueh Awaiting requested review from benjaminfrueh benjaminfrueh is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Database checks: Select_full_join warning is caused by the setup checks' own schema introspection

2 participants

@littleopa @kesselb